-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(PC-31838)[API] feat: add allowed actions for collective offer templates #14242
(PC-31838)[API] feat: add allowed actions for collective offer templates #14242
Conversation
04261fe
to
c3577d3
Compare
Visit the preview URL for this PR (updated for commit 424215a): https://pc-pro-testing--pr14242-pc-31838-eac-back-re-yneuocu5.web.app (expires Sun, 22 Sep 2024 13:43:56 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 032d233ee67e1c50d6af12e29c936c7076770eb1 |
c3577d3
to
7dea139
Compare
7dea139
to
424215a
Compare
@@ -194,6 +203,38 @@ class CollectiveOfferAllowedAction(enum.Enum): | |||
CollectiveOfferDisplayedStatus.INACTIVE: (), | |||
} | |||
|
|||
TEMPLATE_ALLOWED_ACTIONS_BY_DISPLAYED_STATUS: dict[ | |||
CollectiveOfferDisplayedStatus, tuple[CollectiveOfferTemplateAllowedAction, ...] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Les CollectiveOfferTemplateAllowedAction étant de taille variable une liste ou un set me semble plus approprié.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Un tuple a l'avantage de ne pas être mutable, comme ce sont des "constantes" ça me paraissait adapté
Dans ma 1è PR j'étais même tombé sur le piège : c'était une liste, je le récupère dans une méthode, je modifie la liste.. et boum c'est modifié pour tout le monde
@@ -665,3 +669,9 @@ def test_is_two_days_past_end(self): | |||
|
|||
offer.collectiveStock.endDatetime = datetime.datetime.utcnow() - datetime.timedelta(days=3) | |||
assert offer.is_two_days_past_end | |||
|
|||
@pytest.mark.parametrize("status", COLLECTIVE_OFFER_TEMPLATE_STATUS_LIST) | |||
def test_get_offer_template_allowed_actions(self, status): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je ne vois pas vraiement l'intéret de ce test -> il me semble trop proche de l'implementation, le jour ou l'implementation change, ce test devra changer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ça a le mérite d'appeler la méthode directement
Et comme ça boucle sur tous les statuts ça vérifie qu'on en a pas oublié un dans le dico
But de la pull request
Ticket Jira (ou description si BSR) : https://passculture.atlassian.net/browse/PC-31838
Ajout des actions autorisées sur une offre vitrine en fonction du statut.
Envoi au front quand on récupère une offre ou la liste des offres vitrines.
Vérifications